Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read include directories from .ocp-index #79

Closed
wants to merge 9 commits into from
Closed

Read include directories from .ocp-index #79

wants to merge 9 commits into from

Conversation

nojb
Copy link
Contributor

@nojb nojb commented Sep 29, 2015

See #76.

The behavior of ocp-index is left unchanged if no .ocp-index file is found in the tree. If it is, then it used as project root and a list of include directories is read from it. A new flag --no-recursive is added to complement this functionality.

@CLAassistant
Copy link

CLAassistant commented Sep 29, 2015

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@AltGr
Copy link
Member

AltGr commented Oct 2, 2015

Two remarks:

  • the .ocp-index file should be documented in README.md
  • it would be good to prefix the directory names somehow in the file, in case we later want to add some more options to it. The best would be to follow the command-line options format, e.g. I dir,dir.

That's it, thanks a bunch!

@nojb
Copy link
Contributor Author

nojb commented Oct 2, 2015

Agree, will do it later today. Thanks!

@nojb
Copy link
Contributor Author

nojb commented Oct 5, 2015

I have polished the patch; each line of .ocp-index file should be of the form I "<dir>" where "<dir>" follows the lexical conventions of OCaml. With this I think the patch is ready for inclusion; if you agree I will rebase and squash everything into a single commit.

@rgrinberg
Copy link
Contributor

I think this is a nice feature but as a user I'm a bit annoyed that I have to maintain yet another config file with this random metadata.

Would it be possible for ocp-index to simply read build folder directories from .merlin files? For example:
https://github.com/ocaml/ocaml/blob/trunk/.merlin#L2

This example also shows how widely useful this might be as there are already quite a few .merlin's out there.

@AltGr
Copy link
Member

AltGr commented Feb 26, 2016

Oh, sorry @nojb, I had completely forgotten about this, wouldn't it have been for @rgrinberg's remark. He actually has quite a point, if all the information we might need is already available in .merlin files, adding a new file format is superfluous, and I'd be quite happy to re-use them, esp. as it's nicer on the users.

The only downside I can see is that if a project has an out of sync or wrong .merlin, it would hurt ocp-index while the heuristics might have worked; but I don't think it's a worthwile concern.

The good news is that @nojb's contribution would need very little changes to use .merlin files.

@nojb
Copy link
Contributor Author

nojb commented Feb 26, 2016

Sure, sounds like a good idea! I will try to to take a look at this today and make the necessary changes.

Thanks!

@nojb
Copy link
Contributor Author

nojb commented Apr 14, 2016

Just a quick update: I looked at this but it is not so easy because .merlin files supports patterns. We could re-implement the merlin logic in ocp-index but it is not so nice. We could also ask the merlin people to release a tiny library to read .merlin files so that we could reuse that.

@AltGr
Copy link
Member

AltGr commented Apr 29, 2016

Indeed, that may be more complicated than we first thought, and the code seems non trivial.

Also, I didn't read the full spec, but I am not sure .merlin files are necessarily at the root of the project?
Not sure where to go next, thus, .ocp-indent files with syntax a subset of .merlin files (so that users don't have to learn anything new) ? Form a consortium to define a common .project file format with a separate lib ?

@hcarty
Copy link
Contributor

hcarty commented Apr 29, 2016

A shared .project format used across all of these editor/exploration tools would be very nice to have. One format would make it easy enough to create a tool or suite of tools to automatically extract the relevant details from an oasis/ocp-build/obuild/etc build configuration.

@let-def
Copy link

let-def commented Apr 29, 2016

I would be happy to provide a library for reading .merlin files (probably as part of let-def/merlin-extend).
But I am interested in a more generic .project description also.

@XVilka
Copy link

XVilka commented Aug 16, 2019

Should be this either updated or closed?

@nojb
Copy link
Contributor Author

nojb commented Aug 16, 2019

We have since switched to merlin for editor support, so we are not affected by this anymore.

@nojb nojb closed this Aug 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants